Review — PR #809 (round 1)

1. Auto-selected agent is still dropped outside the composer (correctness)

NewSessionComposer.tsx:163-180 corrects the selection when the stored/default agent is
not installed, but that correction is only local state — saveUiPrefs runs solely in
pickHarness (NewSessionComposer.tsx:201-205). So on a machine where Claude Code is
absent, loadUiPrefs().preferredHarness stays undefined and preferredHarness()
(App.tsx:1749) returns "claude-code".

Failure: fresh install, only Codex installed, composer shows Codex. Open templates from
the rail (or the palette, or a ?template=… deep link — none of which get the composer
override) and use any template → createSessionAt(cwd, "claude-code"), i.e. a session on
an adapter the machine does not have. That is the exact symptom in the PR title, just
reached through the entry points the override doesn't cover.

The test matrix shows the gap: template-harness.spec.ts runs the
__MOCK_UNINSTALLED_HARNESSES__ case for composer / gallery-detail / starter-detail
only, while the rail / palette / deep-link loop uses an explicitly picked (therefore
persisted) Codex. One-line fix: persist the corrected id inside the registry effect, which
also repairs App.tsx:1708, :1959 and :2875.

2. Back/forward navigation discards the composer override (correctness)

applyVisit (App.tsx:1152) calls setTemplatesOpen(visit.kind === "templates") with no
second argument, so replaying a templates visit resets templatesView to
{ harness: undefined } and the launch falls back to preferredHarness(). Harmless when
the pick was persisted — but the override exists precisely for the case where it wasn't,
which this PR tests ("preferences cannot be saved"). Sequence: pick Codex with
setItem throwing → browse templates → exit → Back → use a template → Claude Code.
Either carry the harness on the NavigationVisit record or accept it and drop the claim in
the README that the composer selection survives a failed save.

Nits

• README.md:24-27 documents an implementation quirk to npm consumers ("even when
  preferences cannot be saved", "Other gallery entry points use the saved preference"). A
  reader can't act on it; either make the behaviour uniform (finding 1) or cut the sentence
  to "templates launch with your selected coding agent".
• The same hardcode the PR removes is still in the neighbouring clone/start flows —
  App.tsx:2055, :2481, :2537, :3812, :3875 — where a user who picked Codex hits
  the identical surprise. Out of scope here; worth a follow-up ticket rather than scope creep.
• setTemplatesOpen changed from a useState setter to a useCallback; only the dep array
  at App.tsx:1220 was updated, applyVisit's was not. Both are stable so nothing breaks,
  and eslint src --ext .ts never sees web/src — flagging only so the inconsistency isn't
  taken as intent.

Verdict

Approach is right (derive templatesOpen from a per-visit record; thread the selection
through the call), changeset level is correct for a behaviour fix, and no confidentiality or
published-surface problems: the changeset, README and test fixtures name only Sapiom and the
public adapters. Finding 1 leaves the headline bug reachable on an install without Claude
Code — worth fixing before merge; finding 2 is a small follow-up.

wouldn't it be better to have a global state that tracks the harness that you chose instead of passing it to the template creation ? so that we don't have same issue going forward and have only 1 source of truth ?

Review — PR #809 (round 2)

Delta reviewed: adfee74c → bba398d4 (the round-1 marker sha is the merge commit for
adfee74c; the two new commits lift the selection into App).

Both round-1 findings are fixed: the registry correction now lives in App.tsx:259-272 so
every entry point (rail, palette, deep link) sees it, and NavigationVisit carries
harness so Back/Forward replays the gallery visit's selection. The new tests cover both,
including the uninstalled-Claude-Code case for rail/palette/deep-link. README nit and the
applyVisit dep-array nit are addressed too.

1. NEW — a failed registry fetch turns template launches into a no-op

App.tsx:2149: const registry = harnessEntries ?? (await harness.listHarnesses()). The
App-level effect swallows its own failure (.catch(() => {}), :270), so after a failed
probe harnessEntries stays null and every template use re-fetches. If that call rejects,
handleUseTemplate now throws before creating anything, and the composer caller drops it —
void handleUseTemplate(...) at :2267 has no catch. Failure: adapter-probe endpoint
5xx/timeout while sessions still create fine → clicking a template in the composer does
nothing, no toast, no session. Before this push it launched with the selected harness.
Fix: await harness.listHarnesses().catch(() => FALLBACK_HARNESSES) (already imported).

2. Round-1 nit 2 — still two sources of truth inside one component

preferredHarness() (App.tsx:1788) still reads persisted prefs, and the auto-correction
is never persisted, so :1747, :1998, :2922 and :386 keep booting claude-code on a
machine without it while the composer displays Codex. Out of scope in round 1; now that
selectedHarness is in the same scope it is a one-argument change, and it is what the
human reviewer asked for.

Nothing else new; no confidentiality or published-surface problems in the delta (prose
names only Sapiom and public adapters, __MOCK_UNINSTALLED_HARNESSES__ follows the
existing __MOCK_* idiom in api.ts, changeset unchanged and still correctly a patch).

Verdict: finding 1 is worth a one-line guard before merge; the rest of the push
resolves round 1.

Updated in f608e54: selectedHarness is now the single App-level source of truth. I removed the template/navigation snapshots and the remaining launch-time preference reads. Back/Forward uses the current choice, including when saving fails. The registry-fetch failure is handled too; all 71 focused browser tests pass.

On the earlier dependency-array nit, the flagged array already belonged to applyVisit and included setTemplatesOpen. The neighboring explicit Claude clone/start choices predate this template fix.

Rebased on 6ab81d9. Node 20 CI fails on the archive regression; the same failure reproduces on exact main.

The full browser CI run caught a test that changed local storage after App initialized. Updated it in 876d252 to choose Codex through the UI, keeping the project-plus assertion intact. The prior run passed the other 621 tests; CI is rerunning.

The coding agents is good.

but the backfill stops after 200 conversations, but cleanup still runs afterward. If more than 200 need archiving, cleanup can delete the remaining conversations' raw events before they're archived, e.g. when they're older than 30 days. We should finish archiving the remaining batches before cleanup, and keep the source if archiving fails.

@ratataque thanks again, kept the 200-record limit to control startup writes. Cleanup now waits until archiving is complete. If a pass reaches the limit or fails, we keep the source events and continue on the next scheduled pass.

added tests for 200+ conversations, failures, and archive eviction between passes as well